bundler: make --keep-names preserve .name via __name() runtime calls - #35307

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/js_parser/visit/visit_stmt.rs:874-884 — For an anonymous export default class {} with decorators (or a bare accessor field), the decorator-name-injection at line 830 has already set class.class.class_name = Some(data.default_name) by the time keep_name is computed here, so load_name_from_ref returns the bundler-internal {source}_default symbol name (e.g. "d_default") instead of hitting the DEFAULT_ALIAS fallback. The sibling SFunction arm at lines 584-589 avoids this by computing name before injecting data.default_name; capturing class_name before line 824 (or falling through to DEFAULT_ALIAS when n.ref_ == data.default_name.ref_) would fix it.

  Extended reasoning...

  What the bug is

  The export default class keep-names block computes keep_name from class.class.class_name at line 875, but by that point the pre-existing decorator-name-injection at lines 824-832 may have already overwritten it with data.default_name:

  // lines 824-832 (pre-existing)
  if class.class.has_decorators || class.class.should_lower_standard_decorators {
      if class.class.class_name.is_none() || ...ref_.is_empty() {
          class.class.class_name = Some(data.default_name);
      }
  }
  // ...
  // line 835 — follow-up commit 72e69dfc captures has_custom_static_name here,
  //            but class_name has ALREADY been injected above
  // ...
  // lines 875-877 (new in this PR)
  let keep_name: &[u8] = match class.class.class_name {
      Some(n) if !n.ref_.is_empty() => p.load_name_from_ref(n.ref_),
      _ => js_ast::ClauseItem::DEFAULT_ALIAS,
  };

  data.default_name.ref_ is a real symbol created by create_default_name() (p.rs:3995) whose original_name is the bundler-internal identifier "{source_path_ident}_default" — not the string "default". So the match takes the Some(n) if !n.ref_.is_empty() arm, load_name_from_ref (p.rs:756-763) returns symbols[...].original_name.slice(), and the DEFAULT_ALIAS fallback is never reached.

  Code path that triggers it

  Any anonymous export default class {} whose has_decorators or should_lower_standard_decorators is set. Per the comment at lines 820-823, that includes classes with only an auto-accessor field and no decorators at all:

  // d.ts, --keep-names --minify-identifiers
  export default class { accessor x = 1 }
  // or with experimentalDecorators:
  // @dec export default class {}

  Why existing code doesn't prevent it

  The follow-up commit 72e69df moved the has_custom_static_name capture to line 835 (before lower_class), but that is still after the injection at line 830 and does not capture the pre-injection class_name. The earlier inline comment on this PR about lower_class()/mem::take() mutations covers different mutation sites (lines 841 and 868); the injection at 830 is a third, earlier one it does not address.

  Contrast with the sibling SFunction arm at lines 584-589, which computes name from func.func.name in the same expression that injects data.default_name, so an anonymous export default function() {} correctly gets DEFAULT_ALIAS. The SClass arm is asymmetric.

  Step-by-step proof

  1. Input: /d.ts = export default class { accessor x = 1 } with --keep-names --minify-identifiers.
  2. parse_stmt.rs creates data.default_name via create_default_name(), whose symbol original_name is "d_default" (from the source path).
  3. visit_stmt.rs:824: should_lower_standard_decorators is true (auto-accessor lowering), class_name is None.
  4. Line 830: class.class.class_name = Some(data.default_name) — now class_name points at the d_default symbol.
  5. Line 875: match class.class.class_name sees Some(n); n.ref_ is a real symbol so !n.ref_.is_empty() is true.
  6. p.load_name_from_ref(n.ref_) returns symbols[inner_index].original_name.slice() = b"d_default".
  7. Line 880 emits __name(default_ref, "d_default").
  8. At runtime, the imported class's .name === "d_default" — a leaked internal bundler identifier — instead of the spec value "default" (which the KeepNamesExportDefault test asserts for the non-decorated /d.js case).

  Impact

  Narrow: opt-in flag × anonymous export default class × decorators/accessor. Not a regression relative to before this PR (where .name was already wrong under --minify-identifiers), but it's newly-added logic emitting the wrong string for a case whose sibling arm handles it correctly — REVIEW.md's "Fix the whole class — parallel switch arms".

  Fix

  Mirror the SFunction arm: capture keep_name from class.class.class_name before line 824. Alternatively, in the match at 875, fall through to DEFAULT_ALIAS when n.ref_ == data.default_name.ref_. Optionally add a decorated/accessor variant to KeepNamesExportDefault.

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/js_parser/visit/visit_stmt.rs:838-841 — The keep_name capture runs after the decorator-name-injection block at lines 824-832, so for an anonymous export default @dec class {} (or export default class { accessor x }) it reads the synthesized data.default_name ref and emits __name(ref, "<src>_default") instead of __name(ref, "default") — leaking an internal identifier as the runtime .name where the spec (§16.2.3.7) mandates "default". Move this capture above line 824 (or add if n.ref_ != data.default_name.ref_ to the Some(n) guard) so the anonymous case falls through to DEFAULT_ALIAS.

  Extended reasoning...

  What the bug is

  Commit 05f92bc fixed the earlier lower_class/mem::take ordering issue by capturing keep_name and has_custom_static_name before lower_class() runs. But it placed that capture at lines 838-841, which is still after the pre-existing decorator-name-injection block at lines 824-832. That block gives an anonymous export default class a synthetic name whenever decorator lowering needs one:

  // visit_stmt.rs:824-832
  if class.class.has_decorators || class.class.should_lower_standard_decorators {
      if class.class.class_name.is_none() || class.class.class_name.unwrap().ref_.is_empty() {
          class.class.class_name = Some(data.default_name);
      }
  }

  So by the time the keep_name match runs, class.class.class_name is Some(data.default_name) for an anonymous decorated class, and the Some(n) if !n.ref_.is_empty() arm is taken instead of the intended _ => DEFAULT_ALIAS fallback.

  Code path

  data.default_name for an anonymous export default class is created via create_default_name (p.rs:3995-4009), whose symbol original_name is format!("{}_default", source.path.name().fmt_identifier()). load_name_from_ref on a RefTag::Symbol ref (p.rs:761-763) returns that original_name slice. So for a source file d.js, keep_name becomes b"d_default", and line 881 emits __name(default_ref, "d_default").

  Per ES §16.2.3.7 (NamedEvaluation of ClassDeclaration : class ClassTail), an anonymous export default class {} gets .name === "default". The _ => js_ast::ClauseItem::DEFAULT_ALIAS arm (b"default") is clearly what was intended for this case — it just never fires when decorator lowering has already injected a name.

  Why existing tests don't catch it

  KeepNamesExportDefault's /d.js is export default class {} with no decorators or auto-accessors, so has_decorators and should_lower_standard_decorators are both false, line 830 doesn't fire, class_name stays None, and the DEFAULT_ALIAS arm is correctly taken. Adding a decorator or an accessor field to /d.js would make the test assert "default" and observe "d_default".

  Step-by-step proof

  1. Input: export default @dec class {} (or export default class { accessor x = 1 }) with --keep-names.
  2. Parse: class_name = None; has_decorators = true (or should_lower_standard_decorators = true); data.default_name = create_default_name(...) with original_name = "<src>_default". Line 816-818 ensures data.default_name.ref_ is a real symbol.
  3. visit_stmt.rs:824-832: condition true, class_name.is_none() → class.class.class_name = Some(data.default_name).
  4. visit_stmt.rs:838-841: match class.class.class_name { Some(n) if !n.ref_.is_empty() => p.load_name_from_ref(n.ref_) } → "<src>_default".
  5. visit_stmt.rs:881: keep_stmt_symbol_name(stmt.loc, data.default_name.ref_, "<src>_default") → emits __name(ref, "<src>_default").
  6. Runtime: .name === "d_default" (or "entry_default", etc.); expected "default".

  Not a duplicate

  • The resolved lower_class/mem::take thread is what this capture was added to fix; this bug was introduced by that fix's placement.
  • The open comment at line 1112 (decorator-suffix ordering) is about when __name runs relative to decorator callbacks, not what string it carries.
  • The open e_class/should_lower_standard_decorators comment is about the class-expression path in visit_expr.rs.

  Impact & fix

  Narrow — requires anonymous export default class + decorators/auto-accessors + opt-in --keep-names — and not a regression (the flag was a no-op before this PR). But it leaks an internal synthesized identifier into user-visible .name from newly-added logic whose intent (the DEFAULT_ALIAS arm) is defeated. One-line fix: hoist the keep_name/has_custom_static_name capture above line 824, or add if n.ref_ != data.default_name.ref_ to the Some(n) guard so the injected default-name ref falls through to DEFAULT_ALIAS.

No new issues found in this revision — the earlier rounds' feedback (tree-shaking flag, decorator-lowering ordering, simple parameter defaults, bundle-functions keepNames) is all addressed. I'm deferring rather than approving because this is a ~400-line feature implementation across six parser/visitor files with restructured control flow in e_class, and it activates the previously-inert --keep-names in bundle-modules.ts (release-build internal modules now get __name() calls) — worth a human sanity check on the size/perf side.

What was reviewed:

• visit_binary.rs: &&= added to the logical-assign arm; verified the {}.field ??= v HMR fold stays gated off for &&= (correct — {}.x &&= v is undefined, not v).
• e_class restructure: early-return removed so the __name() wrap runs on the lowered-decorator result too; has_custom_static_name and original_class_name are captured pre-lowering.
• s_class/s_function/export-default: __name insertion order relative to lowered decorator suffix and namespace assignments.
• bundle-functions.ts now drops keepNames entirely (no injected helper, no sloppy-branch hole).

No new issues found in this pass, but this is a ~400-line parser/visitor change implementing a user-facing bundler feature with knock-on effects on internal-module codegen, so it warrants a human sign-off.

What was reviewed:

• keep_expr_symbol_name / keep_stmt_symbol_name emitters and all call sites (declarators, assignments incl. &&=/||=/??=, param defaults, export default, TS namespaces, s_class decorator ordering).
• e_class control-flow restructure (early return → else) and the class_has_custom_static_name skip.
• bundle-functions.ts dropping keepNames — the sibling bundle-modules.ts still passes --keep-names, so release-build internal modules now run __name() (userland Object.defineProperty) at load; the author flagged this as a possible follow-up.
• comment-cop has 7 open inline notes on the new code comments.